fix(review): gate the tuning advisor on wouldMerge and weighted precision, matching the breaker - #10142
Conversation
…sion, matching the breaker computeTuningRecommendations disagreed with its sibling circuit breaker on both rules the breaker documents. It gated the sample on decided (holds included) instead of wouldMerge, so 9 holds + 1 wrong would-merge cleared decided>=10, produced mergePrecision 0, and emitted a warn carrying an auto-applicable overridePayload -- queuing a live confidence-floor raise off a single prediction the breaker already refuses. And it read the RAW mergePrecision/closePrecision while the breaker gates on the reversal-WEIGHTED fields, so a project whose merges are systematically reverted (weighted ~0, raw healthy) got no tightening recommended -- exactly the project that needs one -- and evaluateShadowPromotion dropped its queued tightening on the raw number the breaker itself distrusts. Gate the advisor's sample on wouldMerge < MIN_DECIDED, its risk/ready tests on weightedMergePrecision/weightedClosePrecision, and its close-side warn on the weighted close precision against AUTOTUNE_CLOSE_PRECISION_FLOOR (closeFalse stays in the message, no longer the condition). Thread the weighted merge precision into evaluateShadowPromotion by renaming AutoApplyContext.mergePrecision to weightedMergePrecision. Reorder planCloseAutoTune's guard to null-check first, matching planAutoTune, so its formerly-dead null arm is reachable. Every threshold constant, severity ordering, and the tightening-only direction are unchanged. Closes JSONbored#10014
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-31 09:33:26 UTC
Review summary Nits — 5 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10142 +/- ##
===========================================
- Coverage 91.99% 80.02% -11.97%
===========================================
Files 931 284 -647
Lines 114000 58944 -55056
Branches 27523 8754 -18769
===========================================
- Hits 104877 47172 -57705
- Misses 7823 11480 +3657
+ Partials 1300 292 -1008
Flags with carried forward coverage won't be shown. Click here to find out more.
|
What & why
src/review/auto-tune.tshas two consumers of the sameGateEvalReport: the circuit breaker (planAutoTune) and the tuning advisor (computeTuningRecommendations, whose output feeds the auto-apply/override path). They disagreed on both rules the breaker documents.1. Sample gate:
decidedvswouldMerge. The advisor gated onr.decided < MIN_DECIDED;decidedcounts every prediction with a known outcome, holds included. So the exact shapeplanAutoTune's comment names — 9 holds + 1 wrong would-merge — clearsdecided >= 10, producesmergePrecision === 0, and emits awarncarrying anoverridePayload. That payload is auto-applicable:runAutoApplyRecommendationsqueues a live confidence-floor raise off a single prediction the breaker already refuses as statistically meaningless.2. Precision field: raw vs weighted.
GateEvalRow's doc states the breaker gates on the reversal-discountedweighted*fields "so a high volume of later-reverted merges cannot keep the raw number artificially healthy while gaming the breaker into staying disengaged." The advisor read the rawmergePrecision/closePrecision/closeFalse.REVERSAL_DISCOUNT_WEIGHTis0, so a project whose merges are systematically reverted hasweightedMergePrecision ≈ 0whilemergePrecisionstays healthy — the breaker engages, the advisor stays silent, and no tightening is recommended for exactly the project that needs one. Worse:evaluateShadowPromotion's recovery check was fed the raw number, so a project held by the reversal-weighted breaker had its pending tightening dropped on a figure the breaker itself distrusts.3. Dead guard arm.
planCloseAutoTune's guard wasr.wouldClose < AUTOTUNE_MIN_DECIDED || r.weightedClosePrecision == null— butweightedClosePrecisionis non-null iffwouldClose > 0, so oncewouldClose >= 10the== nulldisjunct was unreachable (a dead branch under branch-counted coverage).The fix
computeTuningRecommendations's sample gate isr.wouldMerge < MIN_DECIDED(message names the would-merge count); its merge-risk and ready tests readr.weightedMergePrecision; its close-side warn readsr.weightedClosePrecisionagainstAUTOTUNE_CLOSE_PRECISION_FLOOR(closeFalsestays in the message text, no longer the condition).AutoApplyContext.mergePrecisionis renamed toweightedMergePrecisionand threaded intoevaluateShadowPromotion'scurrentMergePrecision, so the recovery check reads the weighted number.planCloseAutoTune's guard is reordered to null-check first (mirroringplanAutoTune), making the null arm reachable.Unchanged: every threshold constant (
MIN_DECIDED10,RISK_MERGE_PRECISION0.9,READY_MERGE_PRECISION0.95,READY_CLOSE_PRECISION0.9,AUTOTUNE_CLOSE_PRECISION_FLOOR0.85,TIGHTEN_FLOOR_TARGET), thewarn/good/infoordering, theTuningRec/OverridePayloadshapes,isStrictlyTightening's tightening-only direction, andplanAutoTune/applyAutoTune/shouldAutoClearand their close twins.Tests (
test/unit/auto-tune.test.ts,test/unit/auto-apply.test.ts)decided: 10,wouldMerge: 1) isinfo-only with nooverridePayload; a healthy raw merge precision (0.98) with a failing weighted one (0.2) warns withoverridePayload: { confidenceFloor: 0.95 }.planCloseAutoTune's reordered null arm is reachable (weightedClosePrecision: null,wouldClose: 20→ no action).closeFalsebehaviour are updated to the weighted-gate contract; the shadow-promotion recovery tests now passweightedMergePrecision.main.Validation
src/review/auto-tune.tsandsrc/review/auto-apply.tsis 100% line and branch.npm run typecheckclean for these files (the rename is caught end-to-end — the sole call site,selftune-wire.ts, does not pass the field, and the test call sites are updated);npm run engine-parity:drift-checkpasses (neither file is a twin);npm run dead-exports:checkclean; both suites (158 tests) green.git diff --checkclean; no schema/migration/generated-artifact change.Closes #10014